Skip to content

Conversation

Andrew-S-Rosen
Copy link
Member

@Andrew-S-Rosen Andrew-S-Rosen commented Jul 20, 2025

Closes #4456.

Summary

The PBE_64 directory name was set to POT_PAW_PBE_64 instead of POT_GGA_PAW_PBE_64 in pymatgen.io.vasp.inputs even though the Pymatgen CLI uses the latter name. This PR fixes the naming so that it matches what is produced from the CLI, which is consistent with the older POTCAR versions as well.

EDIT: The new CLI as of 2025.6.14 uses POT_PAW_PBE_64, but this is a breaking change compared to the CLI behavior before 2025.6.14. This PR reverts the name change.

Closes #4456.

Signed-off-by: Andrew S. Rosen <[email protected]>
@Andrew-S-Rosen Andrew-S-Rosen changed the title Fix PBE_64 folder Fix PBE_64 folder directory name in pymatgen.io.vasp.sets Jul 20, 2025
@Andrew-S-Rosen Andrew-S-Rosen changed the title Fix PBE_64 folder directory name in pymatgen.io.vasp.sets Fix PBE_64 folder directory name in pymatgen.io.vasp.inputs Jul 20, 2025
@Andrew-S-Rosen
Copy link
Member Author

Andrew-S-Rosen commented Jul 20, 2025

FYI, letting you know @esoteric-ephemera.

@esoteric-ephemera
Copy link
Contributor

Did this not get fixed in #4431?

@Andrew-S-Rosen
Copy link
Member Author

Andrew-S-Rosen commented Jul 23, 2025

@esoteric-ephemera: Thanks for the redirect.

Yes, it looks like if the user installs Pymatgen today and runs the CLI entirely from scratch, everything should work due to #4431.

Upon further inspection, it seems like the issue @jinlhr542 and I ran into in #4456 was due to the breaking change. In previous versions of Pymatgen, the CLI would make a folder called POT_GGA_PAW_PBE_64. For whatever reason, this is now called POT_PAW_PBE_64. So, users who want to use the .64 PAW PBE potentials with Pymatgen or Atomate2 but used the Pymatgen CLI to generate the pseudopotential directory before #4431 was merged will run into compatibility issues.

@Andrew-S-Rosen
Copy link
Member Author

Andrew-S-Rosen commented Jul 23, 2025

@esoteric-ephemera: So I guess the question is --- do we revert the breaking name change, or do we just stick with what we have in master?

@JaGeo
Copy link
Member

JaGeo commented Jul 23, 2025

(I would, as usual, be happy about avoiding breaking changes. 😅)

@esoteric-ephemera
Copy link
Contributor

Either way leads to a breaking change, since a user who set up the directory structure manually and doesn't use the CLI (like me) will have it break the other way

Only non breaking solution would be a check for either directory name

@Andrew-S-Rosen
Copy link
Member Author

@esoteric-ephemera: Yes, this is also true.

I am okay with keeping things as is if that's preferred, but I want to document it here in case we get other reports. I will go ahead and close this PR, but anyone can comment if they want it re-opened.

@esoteric-ephemera
Copy link
Contributor

Hey sorry I was unclear, I'm happy to add this check for either PBE.64 folder name. It's probably for the best if both are recognized by pymatgen

@Andrew-S-Rosen
Copy link
Member Author

@esoteric-ephemera: That works and may reduce the headache in the long-term for MP staff 😉

Thanks for clarifying and offering to help! Certainly feel free to do so if you feel so inclined!

@Andrew-S-Rosen Andrew-S-Rosen marked this pull request as draft July 24, 2025 02:37
@shyuep
Copy link
Member

shyuep commented Jul 26, 2025

Looks good to me.

@Andrew-S-Rosen
Copy link
Member Author

@shyuep FYI, I wouldn't merge as-is yet. @esoteric-ephemera suggested supporting both names. I'll leave that for others to deal with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong directory name for the PBE_64 POTCARS?
4 participants